drivers: clock_control: stm32: hide STM32_HSE_CLOCK when HSE is off #97256
Conversation
zephyrbot
requested review from
FRASTM,
GeorgeCGV,
djiatsaf-st,
erwango,
etienne-lms,
gautierg-st,
marwaiehm-st and
nordic-krch
drivers/clock_control/Kconfig.stm32
Outdated
| int "HSE clock value" | ||
| default "$(DT_STM32_HSE_CLOCK_FREQ)" if "$(dt_nodelabel_enabled,clk_hse)" | ||
| default 8000000 | ||
| depends on $(dt_nodelabel_enabled,clk_hse) |
There was a problem hiding this comment.
This change would not match the 2 places (2 boards) that set CONFIG_CLOCK_STM32_HSE_CLOCK:
boards/oct/osd32mp1_brk
boards/st/stm32mp157c_dk2
Both do not enable the the clk_hse node.
Note that from what I see, the config is never used. Am I right? Is it still relevant? Maybe to be removed?
There was a problem hiding this comment.
Note that from what I see, the config is never used. Am I right? Is it still relevant? Maybe to be removed?
My mistake, this config is used by the STM32 HAL.
I think it should be changed to a hidden config and always set from the DT when HSE is enabled.
There was a problem hiding this comment.
This change would not match the 2 places (2 boards) that set
CONFIG_CLOCK_STM32_HSE_CLOCK: boards/oct/osd32mp1_brk boards/st/stm32mp157c_dk2 Both do not enable the theclk_hsenode.
Good catch. Clock generators have been omited from MP15 DTSI - presumably because Cortex-A side controls generators but we're running on Cortex-M - but the HAL nonetheless expects this...
Maybe we can special-case MP15 series here? depends on $(dt_nodelabel_enabled,clk_hse) || SOC_SERIES_STM32MP1X + a comment? What do you think?
There was a problem hiding this comment.
Or we can make the option promptless (i.e., can't be used-configured) if HSE exists and visible otherwise.
Maybe this is a less intrusive change. WDYT?
There was a problem hiding this comment.
Maybe we can special-case MP15 series here (...)
For MP15, it some day it needs HSE, I guess we'll enable the node in the board DT and set the frequency.
Actually, we can already set the frequency rate since HSE oscillator is present but let's enable only if needed.
Or we can make the option promptless (i.e., can't be used-configured) if HSE exists and visible otherwise.
I think it's the best way.
There was a problem hiding this comment.
Ok, I just need to check if it actually works then I'll update.
There was a problem hiding this comment.
Made the symbol promptless when clk_hse is enabled and editable otherwise. Updated the MP15 boards to assign a value through configuration files rather than Kconfig symbols.
Ensure the CONFIG_CLOCK_STM32_HSE_CLOCK symbol cannot be overridden when the value is sourced from Device Tree node "clk_hse" by making it promptless in this case. Signed-off-by: Mathieu Choplain <[email protected]>
Set the HSE clock frequency using the board's defconfig file instead of overriding the definition. Signed-off-by: Mathieu Choplain <[email protected]>
Set the HSE clock frequency using the board's defconfig file instead of overriding the definition. Signed-off-by: Mathieu Choplain <[email protected]>
mathieuchopstm
force-pushed
the
stm32_hse_kconfig_visibility
branch
from
276c35c to
1718bf5
Compare
required to avoid CI failures due to dt_nodelabel_int_prop bug Signed-off-by: Mathieu Choplain <[email protected]>
|
maybe better solution: make HSE node mandatory in DT but take its value even if |
|
4 participants
Ensure the
CONFIG_CLOCK_STM32_HSE_CLOCKsymbol is not visible when the HSE clock is not enabled.Depends on #97215.